Don't copy imported module to filter out its dependencies #826
Conversation
ModuleSpace doesn't keep imported spaces as atoms. It keeps them in a separate collection instead. This allows constructing new instances of the ModuleSpace without filtering original imported space.
Space::visit() is safer because it doesn't exposes references to the atoms which are behind internally mutable smart pointer.
Replacing `&Atom` by `Cow<Atom>` thus allowing passing both reference and value instance.
Add TODOs for tests which are possible if get-deps function is implemented.
This implementation has a safety hole. Say the user borrows an iterator from the space behind a RefCell, then uses the iterator to get references to some atoms, saves those references, and then drops the iterator. The user could then use borrow_mut to mutably access the space and delete those atoms. This would leave the original atom references pointing to garbage. Space::visit is implemented instead and all internal iterator usages are replaced by Space::visit usages.
As to this question, it gets into the behavior expected from the MeTTa language / modules. I was assuming behavior 1 was "correct", based on how things worked before the module separation was added. But a case can be made for either behavior. |
Yes, we discussed it with @Necr0x0Der as well. We still can add atomspace which is an atom and at the same time contains functions which are available for execution from the space which contains this atom. Thus one still can do something like "manual import". Introspection of imported modules is important from my perspective. But it is not critical right now and I hope clarifying this behavior later. |
This PR is replacement of #776 without issue which were caused by old interpreter implementation. New commits start from 4277146
In this PR
ModuleSpaceis introduced which keeps main space and its dependencies separately. This allows querying them separately and when dependent space is queried it doesn't queried recursively. Thus all transitive dependencies are queried only once from the top level space.For example if we have space
Awhich imports spaceBandCandDis imported fromBandCthenAcontainsB,CandDas dependencies in a separate collection. Query toAgoes through its atoms first then goes toB,CandDbut dependencies of these spaces are not queried.ModuleSpace::atom_iter()iterates through main atomspace only. This breaks tests inf1_imports.mettawhich are disabled in this change. There are two ways of solving this: (1) adding dependency spaces as atoms intoModuleSpaceiterator which resembles previous behavior, (2) add separate function which gets dependencies from space. I would like to implement solution after discussing a way. Option (2) is not possible with currentDynSpaceused internally as space representation may be it can be implemented after additional refactoring.New
Space::visitmethod is added because it is impossible to implementSpace::atom_iterforDynSpacesafely (see @luketpeterson comment #776 (comment)). Internal usages ofSpace::atom_iterare replaced bySpace::visitusages.